Skip to content

Conversation

rklaehn
Copy link
Collaborator

@rklaehn rklaehn commented Aug 28, 2025

Description

Refactor provider events into a proper irpc protocol.

Also allow configuring for each event type if the event is sent as a notification, a proper request with answer, or not at all.

Breaking Changes

Provider events completely changed. Other than that the changes should be minimal. You can still create a BlobsProtocol by passing None.

Notes & open questions

Note: to review, best to start with looking at the limit example, then look at the docs.

Change checklist

  • Self-review.
  • Documentation updates following the style guide, if relevant.
  • Tests if relevant.
  • All breaking changes documented.

@n0bot n0bot bot added this to iroh Aug 28, 2025
@github-project-automation github-project-automation bot moved this to 🏗 In progress in iroh Aug 28, 2025
@rklaehn rklaehn force-pushed the provider-events-refactor branch from 1a176ae to b26aefb Compare September 2, 2025 08:01
Copy link

github-actions bot commented Sep 2, 2025

Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh-blobs/pr/142/docs/iroh_blobs/

Last updated: 2025-09-11T14:49:35Z

This shows how to limit serving content in various ways

- by node id
- by content hash
- throttling
- limiting max number of connections
@rklaehn rklaehn marked this pull request as ready for review September 3, 2025 08:43
@rklaehn rklaehn requested a review from Frando September 3, 2025 10:17
@Frando
Copy link
Member

Frando commented Sep 3, 2025

I took a quick look at the examples and the implementation. It looks very solid overall and is a great improvement especially for power-user usecases. I have yet to look closer to comment on the implementation.

I had a closer look at the limits example to see how this will be used. The API is fine, I think!
While going through the example, I made some small tweaks to make the example more DRY to increase readability and focus on the relevant parts. I also added a small constructor function EventSender::channel that creates the tokio channel for you, to reduce what users need to do and know. Here's the diff: b26b408 - Feel free to cherrypick onto this branch!

@Frando
Copy link
Member

Frando commented Sep 3, 2025

My branch had some copy-paste mistakes, sorry for that - fixups are here: 65e5e40

@rklaehn rklaehn force-pushed the provider-events-refactor branch from 68d284d to dc2c177 Compare September 3, 2025 13:12
so the other side can know if reconnecting is OK
@rklaehn
Copy link
Collaborator Author

rklaehn commented Sep 3, 2025

Thanks for the DRYing. Cherry-picked it.

I think I need to change the GetError a bit. Currently it is a mix of an interpretation of what went wrong and just a flat list of cases. It needs to become either the one or the other. Currently it is hard to get the reset error code back from the GetError since it can hide in so many cases.

But I think GetError refactoring is best left for a subsequent PR.

@rklaehn rklaehn force-pushed the provider-events-refactor branch from b84a7cb to f399e2b Compare September 3, 2025 14:22
@@ -0,0 +1,371 @@
/// Example how to limit blob requests by hash and node id, and to add
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is very cool overall!

pub fn tracing(&self, mask: EventMask) -> Self {
use tracing::trace;
let (tx, mut rx) = tokio::sync::mpsc::channel(32);
n0_future::task::spawn(async move {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no task tracking?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I was too lazy to make this a full fledged actor. Not sure if it is worth it here.

@rklaehn rklaehn merged commit a3e5ef3 into main Sep 11, 2025
23 of 24 checks passed
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in iroh Sep 11, 2025
@rklaehn rklaehn deleted the provider-events-refactor branch September 11, 2025 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

3 participants